fix: /receive now handles login from another location by highesttt · Pull Request #211 · beeper/line

highesttt · 2026-07-09T17:35:24Z

No description provided.

indent-zero · 2026-07-09T17:35:27Z

PR Summary

Handles LINE's forced-logout condition on the /operation/receive SSE stream so poll-loop failures caused by another client stealing the session land in StateBadCredentials / UserActionRelogin instead of looping refresh/re-login attempts. Adds visibility into the LINE response body on non-200 SSE responses and hardens nil-checking so recovery paths don't panic when BridgeState or the embedded *database.UserLogin is unset.

pkg/line/sse.go: reads up to 4 KB of the body on non-200 SSE responses and appends it to the returned error (SSE error: <code>: <body>).
pkg/line/errors.go: adds IsRequestNeedLogin (REQUEST_NEED_LOGIN / "code":10004) and folds it into IsLoggedOut. Adds a hasJSONCode helper that anchors the numeric code on a trailing ,/}/whitespace so it does not over-match longer codes like 100040.
pkg/connector/sync.go: replaces the inline SSE-401/403 branch in pollLoop with a new handleReceiveAuthError that checks IsLoggedOut, probes getProfileWithToken to surface the detailed forced-logout envelope, and (a) if the probe succeeds, returns false to let the poll loop simply reconnect with the current token, (b) otherwise attempts recovery only if neither the SSE error nor the probe indicates a forced logout. Nil-guards lc.UserLogin / Bridge / BridgeState in the recovery-failed branch.
pkg/connector/client.go: nil-checks UserLogin.BridgeState before Send, and nil-checks the embedded *database.UserLogin inside saveSessionInvalidated.
Tests: adds pkg/line/sse_test.go for the body-preserving SSE error, plus three poll-loop tests covering the probe-based logout path, the immediate REQUEST_NEED_LOGIN path, and the "profile still valid → don't rotate token" path. Extends errors_test.go with REQUEST_NEED_LOGIN and code-anchor cases ({"code":10004} matches, {"code":100040} does not).

Issues

All clear! No issues remaining. 🎉

2 issues already resolved

IsRequestNeedLogin uses strings.Contains(msg, "\"code\":10004"), which also matches any 5-digit LINE code in the range 100040–100049; if LINE ever surfaces such a code it will be misclassified as a forced logout and force the user to Reconnect. Anchor on a trailing ,/}/whitespace like the neighbouring hasResponseErrorCode / IsInvalidSenderKey helpers do. (fixed by commit 4943360)
handleReceiveAuthError ignores the nil return from the getProfileWithToken probe: when the profile call succeeds (proving the current token is still valid), the handler still falls through to recoverToken, which rotates the access token and invalidates the OBS token cache. Gate the recovery path on profileErr != nil so spurious /operation/receive 401s don't churn otherwise-valid tokens. (fixed by commit 4943360)

CI Checks

Waiting for CI checks...

coderabbitai · 2026-07-09T17:35:49Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 86091b30-6de7-48e7-bf3f-6a740f14923b

📥 Commits

Reviewing files that changed from the base of the PR and between 0b8a585 and 4943360.

📒 Files selected for processing (4)

pkg/connector/sync.go
pkg/connector/sync_test.go
pkg/line/errors.go
pkg/line/errors_test.go

Disabled knowledge base sources:

Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Improved logout handling for failed receive-auth flows, including correct relogin signaling and safer session invalidation (avoids crashes in edge cases).
- Enhanced authorization detection, including “request need login” patterns and logged-out classification.
- SSE non-OK errors now include a short response-body snippet for clearer diagnostics.
Tests
- Added/expanded tests covering forced-logout behavior, immediate stop on “REQUEST_NEED_LOGIN,” valid-profile no-recovery behavior, and SSE error body inclusion.

Walkthrough

This PR refines LINE SSE authentication-failure handling: error classification now recognizes REQUEST_NEED_LOGIN, SSE errors include limited response bodies, pollLoop delegates auth handling to handleReceiveAuthError, and session-invalidation paths add nil guards.

Changes

SSE Auth-Failure Handling

Layer / File(s)	Summary
REQUEST_NEED_LOGIN detection `pkg/line/errors.go`, `pkg/line/errors_test.go`	Adds `IsRequestNeedLogin` to detect `request_need_login` or code `10004`, and wires it into `IsLoggedOut` with expanded test coverage.
SSE error body inclusion `pkg/line/sse.go`, `pkg/line/sse_test.go`	`ListenSSE` reads up to 4096 bytes from non-200 response bodies and appends trimmed body text to the returned error; a test verifies the body detail is included.
Centralized receive-auth handling in pollLoop `pkg/connector/sync.go`, `pkg/connector/sync_test.go`	Replaces inline unauthorized-string handling with `line.IsUnauthorizedStatus` and `handleReceiveAuthError`, which probes the profile, attempts token recovery, and updates bridge state; tests cover logged-out, REQUEST_NEED_LOGIN, and valid-profile paths.
Nil-safety in session invalidation `pkg/connector/client.go`	`markLoggedOutByOtherClient` now checks `BridgeState != nil` before sending, and `saveSessionInvalidated` returns early when `UserLogin` is nil.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No pull request description was provided, so there is no meaningful description to evaluate.	Add a brief description of the auth-handling changes and their user impact.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: /receive now handles login/logout-related auth failures.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch highest/receive-logout-support

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/connector/sync.go`:
- Around line 1279-1294: Add a nil guard for lc.UserLogin before dereferencing
it in the recoverToken error path inside the poll-loop logic. In the branch that
logs “Failed to recover session, stopping poll loop” and sends BridgeState
updates, first check lc.UserLogin != nil (or return early if it is nil),
matching the defensive pattern used by markLoggedOutByOtherClient in client.go.
Keep the existing errRecover handling, but make the logger and BridgeState
access conditional on a valid UserLogin to avoid a panic when recoverToken fails
while the client has no UserLogin.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e15fda62-23e7-4d48-9014-fd4e7f843f3c

📥 Commits

Reviewing files that changed from the base of the PR and between f4cc52b and 0b8a585.

📒 Files selected for processing (7)

pkg/connector/client.go
pkg/connector/sync.go
pkg/connector/sync_test.go
pkg/line/errors.go
pkg/line/errors_test.go
pkg/line/sse.go
pkg/line/sse_test.go

📜 Review details

⏰ Context from checks skipped due to timeout. (4)

GitHub Check: build-docker
GitHub Check: Lint with 1.25
GitHub Check: build-docker
GitHub Check: Lint with 1.25

🧰 Additional context used

📓 Path-based instructions (2)

**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use go fmt for code formatting across all Go files
Use goimports with -local "github.com/highesttt/matrix-line-messenger" flag to group project-local imports correctly
Use zerolog for logging throughout the codebase
Do not use Msgf in logging; use Msg with structured fields instead
Use Stringer interface where applicable in Go code

Files:

pkg/line/sse.go
pkg/line/errors_test.go
pkg/line/errors.go
pkg/line/sse_test.go
pkg/connector/sync.go
pkg/connector/client.go
pkg/connector/sync_test.go

**/!(ltsm)/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/!(ltsm)/**/*.go: Run staticcheck on all Go files excluding pkg/ltsm package (transpiled WASM code)
Run go vet on all Go files excluding pkg/ltsm package (transpiled WASM code)

Files:

pkg/line/sse.go
pkg/line/errors_test.go
pkg/line/errors.go
pkg/line/sse_test.go
pkg/connector/sync.go
pkg/connector/client.go
pkg/connector/sync_test.go

🔇 Additional comments (10)

pkg/line/errors.go (2)

27-34: LGTM!

47-56: 🎯 Functional Correctness

Fragile substring matching for the JSON code field; prefer the shared helper or a regex-based check.

pkg/line/errors_test.go (1)

57-59: LGTM!

pkg/line/sse.go (2)

7-7: LGTM!

Also applies to: 18-19

50-56: 🔒 Security & Privacy | ⚡ Quick win

Confirm upstream error bodies won't leak sensitive data into logs.

The raw (trimmed) response body is now embedded directly into the returned error, which later flows into .Err(err).Msg("SSE Disconnected") in pollLoop. If LINE's error envelope ever echoes back tokens or other account-identifying data on auth failures, that content would end up in application logs. The 4096-byte cap limits volume but not content.

Consider stripping/redacting known sensitive substrings (e.g., tokens) before including the body, or confirm LINE's error payloads for this endpoint are limited to generic error codes/messages.

pkg/connector/sync.go (2)

1250-1254: LGTM!

1261-1278: LGTM!

Also applies to: 1295-1297

pkg/connector/sync_test.go (1)

5-5: LGTM!

Also applies to: 65-115, 117-140

pkg/connector/client.go (1)

290-303: LGTM!

pkg/line/sse_test.go (1)

11-39: 🎯 Functional Correctness

Check roundTripFunc is available in pkg/line

This test depends on roundTripFunc; if it isn’t defined in another pkg/line helper, the test won’t compile.

…ceeds

fix: /receive now handles login from another location

0b8a585

indent-zero Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread pkg/line/errors.go Outdated

indent-zero Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread pkg/connector/sync.go Outdated

coderabbitai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread pkg/connector/sync.go

fix: linked 10004 to REQUEST_NEED_LOGIN + reconnect if getProfile suc…

4943360

…ceeds

highesttt merged commit d70e29b into main Jul 9, 2026
9 of 10 checks passed

highesttt deleted the highest/receive-logout-support branch July 9, 2026 18:10

This was referenced Jul 9, 2026

fix: bad_credentials after 150s of inactivity #212

Merged

Highest/fix reconnect #214

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: /receive now handles login from another location#211

fix: /receive now handles login from another location#211
highesttt merged 2 commits into
mainfrom
highest/receive-logout-support

highesttt commented Jul 9, 2026

Uh oh!

indent-zero Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

Review failed

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

highesttt commented Jul 9, 2026

Uh oh!

indent-zero Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

indent-zero Bot commented Jul 9, 2026 •

edited

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading